Skip to content

cloud/C6: revocation cache#222

Closed
blueberrycongee wants to merge 2 commits intoloop/cloud-C5from
loop/cloud-C6
Closed

cloud/C6: revocation cache#222
blueberrycongee wants to merge 2 commits intoloop/cloud-C5from
loop/cloud-C6

Conversation

@blueberrycongee
Copy link
Copy Markdown
Owner

What

Implements the daily-refreshed revocation cache from CONTRACT.md §2.5. Public surface is createRevocationCache(opts) for testability plus a default singleton isRevoked(lid) / refreshRevocations() wired to the real C5 HTTP client.

Stacked on #221 (C5) because the cache calls getRevocations. Order: #217 (C1) → #221 (C5) → this PR.

Design

The cache is intentionally fail-open:

  • Storage read throw → treat as cold cache, refresh.
  • Network refresh throw → keep serving the stale cache. Cold-and-network-down resolves to false (not revoked).

Rationale: revocation is defense-in-depth on top of expires_at. Locking users out of cloud features during a transient outage is a worse failure than briefly missing a freshly-revoked id. The signed expires_at boundary still holds.

Concurrent refreshes coalesce into a single fetch via an inFlight Promise — prevents N parallel /v1/license/revocations calls during first paint.

fetchFresh receives the previous as_of as the since argument, matching the contract's ?since=<iso> shape for incremental polling.

Why the default storage is in-memory only

The C6 spec calls for persisting under app.getPath('userData')/lumina-cloud-revocations.json. That path requires the same Electron-IPC plumbing question that has C3 blocked (#219). Rather than block C6 too, I shipped the cache logic with an in-memory default RevocationStorage:

  • The factory pattern means swapping in a real disk-backed storage is a one-line change once C3's IPC question lands.
  • Within a session the cache still de-dupes refreshes; across-restart persistence is the only thing missing.
  • All four C6 test cases (cold / warm / expired / network-fail) pass with the injected storage abstraction.

If you want persistence to ship in the same merge as the cache, point me at the same answer as C3 and I'll wire it.

Acceptance criteria

  • Refreshes if cache age > 24h (configurable via ttlMs).
  • Quietly succeeds with stale cache on network failure.
  • Tests: cold cache, warm cache, expired cache, network failure with stale cache.

How I tested

  • npm run typecheck: pass.
  • npm test -- --run src/services/luminaCloud/revocations.test.ts: 7/7 pass.
  • npm test -- --run src/services/luminaCloud/: 28/28 pass on this branch (revocations + C5 client).

Touched files outside src/services/luminaCloud/

  • cloud/TASKS.md only. No other surfaces touched.

Notes for Lead

  • Tests use a deferred-Promise pattern for the coalesce test so the resolver is set before any fetch is invoked. (First attempt timed out because resolveFetch was assigned during fetchFresh's body, after the synchronous test code had already called the no-op stub.)
  • The cache returns false for "no cache + network down" — fail open. Easy to flip to true (fail closed) if you'd rather be conservative; just make getCurrent return the stale-or-null cache and have isRevoked treat a null cache as "revoked".
  • I exported RevocationCacheData, RevocationStorage, RevocationCacheOptions, createRevocationCache, and REVOCATIONS_DEFAULT_TTL_MS from the barrel so consumers (and the future disk-backed storage adapter) can wire the factory cleanly.

blueberrycongee and others added 2 commits April 28, 2026 12:52
Implements a daily-refreshed cache for §2.5 revocations. The work is
factored as createRevocationCache(opts) so tests can inject storage,
fetchFresh, now, and ttlMs cleanly. The default singleton wires this
to the real getRevocations from the C5 client.

Failure modes are intentionally fail-open:
- Storage read throw → treat as cold cache.
- Network refresh throw → keep serving stale data; cold-and-network-
  down resolves to "not revoked". Revocation is defense-in-depth on
  top of expires_at; locking users out on a transient outage would
  be a worse failure than missing a freshly-revoked id for a few
  minutes.

The default storage is in-memory only because persistent storage
needs the same Electron-IPC plumbing question that has C3 blocked
(PR #219). The cache still de-dupes refreshes within a session;
across-restart persistence will follow when C3 unblocks. Drop-in
replacement: pass a different `storage` to createRevocationCache.

Concurrent refresh requests are coalesced into a single fetch via
an inFlight Promise — prevents N parallel requests on first paint.

Tests cover the four cases C6 lists (cold, warm, expired, network-
fail-with-stale) plus cold+network-fail (fail open) and the
coalescing invariant. 7 tests, all pass.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 260c3bf0ff

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines +63 to +65
const next: RevocationCacheData = {
as_of: fresh.as_of,
revoked_lids: fresh.revoked_lids,
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Merge incremental revocations into cached set

This refresh path overwrites the local list with fresh.revoked_lids even though the fetch is made with since (opts.fetchFresh(stale?.as_of)). If the backend treats since as an incremental cursor, older revoked IDs are dropped after the first delta poll (for example, cached ['lic_old'] + response ['lic_new'] becomes only ['lic_new']), so previously revoked licenses can be treated as valid. Preserve existing IDs when applying incremental responses, or avoid using since for snapshot replacement.

Useful? React with 👍 / 👎.

@blueberrycongee
Copy link
Copy Markdown
Owner Author

Closing — re-issued cleanly by integrator.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant